Skip to content

Conversation

@Pouyanpi
Copy link
Collaborator

@Pouyanpi Pouyanpi commented Nov 6, 2025

Description

Added support for extracting reasoning traces and tool calls from LangChain 1.x content blocks format, with automatic fallback to the legacy format for backward compatibility.

The implementation:

  • extracts reasoning from content_blocks with type: "reasoning"
  • extracts tool calls from content_blocks with type: "tool_call"
  • falls back to additional_kwargs and tool_calls attribute for providers using the legacy format

references:

requires:

#1472

@Pouyanpi Pouyanpi added this to the v0.19.0 milestone Nov 6, 2025
@Pouyanpi Pouyanpi self-assigned this Nov 6, 2025
@Pouyanpi Pouyanpi marked this pull request as ready for review November 6, 2025 14:49
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Added support for extracting reasoning traces and tool calls from LangChain 1.x content_blocks format, with automatic fallback to legacy additional_kwargs and tool_calls attribute for backward compatibility.

Key changes:

  • Refactored _store_reasoning_traces() to try content_blocks first, then fall back to additional_kwargs
  • Refactored _store_tool_calls() to try content_blocks first, then fall back to tool_calls attribute
  • Split extraction logic into separate helper functions for better modularity
  • Added comprehensive test coverage with 28 new tests covering edge cases and fallback behavior

Testing:

  • Tests cover both extraction methods, fallback behavior, multiple content blocks, and edge cases
  • Tests verify proper prioritization (content_blocks over legacy format)
  • Tests include both MockResponse and real LangChain AIMessage objects

Confidence Score: 4/5

  • This PR is safe to merge with minor risk from a KeyError edge case
  • Score reflects solid implementation with comprehensive tests and backward compatibility, but deducted one point for the KeyError risk in reasoning extraction when blocks have type "reasoning" but missing the key
  • Pay close attention to nemoguardrails/actions/llm/utils.py line 252 for the KeyError issue

Important Files Changed

File Analysis

Filename Score Overview
nemoguardrails/actions/llm/utils.py 4/5 Added LangChain 1.x content_blocks support with fallback to legacy format. Implementation is solid but has one KeyError risk in reasoning extraction.
tests/test_actions_llm_utils.py 5/5 Comprehensive test coverage for both extraction methods, fallback behavior, and edge cases with MockResponse and real AIMessage objects.

Sequence Diagram

sequenceDiagram
    participant LLM as LLM Call
    participant Store as _store_reasoning_traces/_store_tool_calls
    participant ExtractCB as _extract_from_content_blocks
    participant ExtractLegacy as _extract_from_legacy
    participant CtxVar as Context Variable

    LLM->>Store: response object
    Store->>ExtractCB: try content_blocks first
    alt content_blocks exists
        ExtractCB->>ExtractCB: iterate blocks
        ExtractCB->>ExtractCB: filter by type
        ExtractCB-->>Store: return extracted data
        Store->>CtxVar: set value
    else no content_blocks
        ExtractCB-->>Store: return None
        Store->>ExtractLegacy: fallback to legacy format
        alt legacy format exists
            ExtractLegacy-->>Store: return extracted data
            Store->>CtxVar: set value
        else no legacy format
            ExtractLegacy-->>Store: return None
            Store->>CtxVar: set None or original value
        end
    end
Loading

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Refactored LLM response processing to support LangChain 1.x content blocks format while maintaining backward compatibility with the legacy format.

Key Changes:

  • Extracted reasoning traces from content_blocks with type: "reasoning" (LangChain 1.x standard)
  • Extracted tool calls from content_blocks with type: "tool_call" (LangChain 1.x standard)
  • Implemented automatic fallback to additional_kwargs and tool_calls attribute for providers using legacy format
  • Refactored extraction logic into separate functions for better modularity and testability
  • Added comprehensive test coverage for both new content blocks format and legacy format

Architecture:
The implementation follows a try-first-then-fallback pattern: it first attempts to extract data from the newer content_blocks format, and only if that fails, falls back to the legacy format. This ensures compatibility with both old and new LangChain versions.

Confidence Score: 4/5

  • Safe to merge with one minor defensive check already flagged in previous comments
  • The refactoring is well-structured with good separation of concerns and comprehensive test coverage. The fallback mechanism ensures backward compatibility. One minor issue (missing key existence check in line 252) was already identified in previous comments and should be addressed before merging.
  • No files require special attention beyond addressing the previously flagged issue in nemoguardrails/actions/llm/utils.py:252

Important Files Changed

File Analysis

Filename Score Overview
nemoguardrails/actions/llm/utils.py 4/5 Refactored reasoning and tool call extraction to support LangChain 1.x content blocks with fallback to legacy format. Added proper defensive checks. One minor issue already flagged in previous comments.

Sequence Diagram

sequenceDiagram
    participant LLM as LLM Call
    participant Store as _store_reasoning_traces / _store_tool_calls
    participant CB as Content Blocks Extractor
    participant AK as Additional Kwargs Extractor
    participant CV as Context Variable

    LLM->>Store: response object
    
    alt Reasoning Extraction
        Store->>CB: _extract_reasoning_from_content_blocks
        CB->>CB: Check hasattr(response, "content_blocks")
        alt content_blocks exists
            CB->>CB: Iterate blocks
            CB->>CB: Find type="reasoning" with "reasoning" key
            CB-->>Store: Return reasoning string or None
        else no content_blocks
            CB-->>Store: Return None
        end
        
        alt No reasoning from content_blocks
            Store->>AK: _extract_reasoning_from_additional_kwargs
            AK->>AK: Check hasattr(response, "additional_kwargs")
            AK->>AK: Get "reasoning_content" from dict
            AK-->>Store: Return reasoning or None
        end
        
        Store->>CV: reasoning_trace_var.set(reasoning_content)
    end
    
    alt Tool Calls Extraction
        Store->>CB: _extract_tool_calls_from_content_blocks
        CB->>CB: Check hasattr(response, "content_blocks")
        alt content_blocks exists
            CB->>CB: Collect all type="tool_call" blocks
            CB-->>Store: Return list or None
        else no content_blocks
            CB-->>Store: Return None
        end
        
        alt No tool_calls from content_blocks
            Store->>AK: _extract_tool_calls_from_attribute
            AK->>AK: getattr(response, "tool_calls", None)
            AK-->>Store: Return tool_calls or None
        end
        
        Store->>CV: tool_calls_var.set(tool_calls)
    end
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Collaborator

@tgasser-nv tgasser-nv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, please address the comments on the tests before merging. These auto-generated tests tend to bloat the line-count of tests by checking individual contents of dicts.

These tests basically store and get the same data back in each test via different permutations. Recommend re-writing test to define a dict which is both stored and checked in the assert

@Pouyanpi Pouyanpi force-pushed the feat/support-langchain-v1 branch 2 times, most recently from b214d7e to c47f986 Compare November 24, 2025 12:27
Base automatically changed from feat/support-langchain-v1 to develop November 24, 2025 14:16
@Pouyanpi Pouyanpi force-pushed the feat/llm-call-lcv1 branch 2 times, most recently from d54d59e to b4b9639 Compare November 24, 2025 14:21
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 24, 2025

Greptile Overview

Greptile Summary

This PR adds LangChain 1.x content_blocks support for extracting reasoning traces and tool calls, enabling compatibility with the new standard format while maintaining backward compatibility.

Key Changes:

  • Implements extraction from content_blocks with type="reasoning" and type="tool_call" (LangChain v1 standard)
  • Maintains fallback chain: content_blocks → additional_kwargs → legacy formats
  • Refactors extraction logic into separate functions for better modularity
  • Adds 20+ comprehensive tests covering all extraction methods and edge cases
  • Includes pytest fixture to properly reset context variables between tests

Implementation Quality:

  • Clean separation of concerns with dedicated extraction functions
  • Proper prioritization of extraction methods (v1 standard first, legacy fallback)
  • Comprehensive test coverage including MockResponse and real AIMessage tests
  • All edge cases handled: missing attributes, empty blocks, mixed content types

Confidence Score: 5/5

  • This PR is safe to merge with no identified risks
  • The implementation is clean, well-tested, and maintains full backward compatibility. The fallback chain ensures existing functionality continues to work while adding new LangChain v1 support. Test coverage is comprehensive with 20+ tests covering all code paths and edge cases. The previous review concern about KeyError was addressed through the developer discussion confirming the downstream interface guarantees.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
nemoguardrails/actions/llm/utils.py 5/5 Adds LangChain v1 content_blocks extraction with clean fallback chain for reasoning and tool calls
tests/test_actions_llm_utils.py 5/5 Comprehensive test coverage with 20+ new tests covering all extraction methods and edge cases

Sequence Diagram

sequenceDiagram
    participant LLM as LLM Provider
    participant llm_call
    participant store_reasoning as _store_reasoning_traces
    participant store_tools as _store_tool_calls
    participant context as Context Variables

    LLM->>llm_call: Return response
    llm_call->>store_reasoning: Extract reasoning
    
    alt Has content_blocks
        store_reasoning->>store_reasoning: _extract_reasoning_from_content_blocks()
        store_reasoning->>store_reasoning: Check for type="reasoning"
        alt Found reasoning in content_blocks
            store_reasoning->>context: Set reasoning_trace_var
        else No reasoning in content_blocks
            store_reasoning->>store_reasoning: _extract_reasoning_from_additional_kwargs()
            alt Found in additional_kwargs
                store_reasoning->>context: Set reasoning_trace_var
            else Still no reasoning
                store_reasoning->>store_reasoning: _extract_and_remove_think_tags()
                alt Found <think> tags
                    store_reasoning->>context: Set reasoning_trace_var
                end
            end
        end
    end

    llm_call->>store_tools: Extract tool calls
    
    alt Has content_blocks
        store_tools->>store_tools: _extract_tool_calls_from_content_blocks()
        store_tools->>store_tools: Check for type="tool_call"
        alt Found tool calls in content_blocks
            store_tools->>context: Set tool_calls_var
        else No tool calls in content_blocks
            store_tools->>store_tools: _extract_tool_calls_from_attribute()
            alt Found tool_calls attribute
                store_tools->>context: Set tool_calls_var
            end
        end
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. nemoguardrails/actions/llm/utils.py, line 266-274 (link)

    logic: PR description claims reasoning extraction from content_blocks with type: "reasoning", but implementation only checks additional_kwargs["reasoning_content"]. Missing the content_blocks extraction path mentioned in LangChain v1 docs.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@Pouyanpi Pouyanpi merged commit ea85f78 into develop Nov 24, 2025
16 of 18 checks passed
@Pouyanpi Pouyanpi deleted the feat/llm-call-lcv1 branch November 24, 2025 14:58
Pouyanpi added a commit that referenced this pull request Nov 24, 2025
Pouyanpi added a commit that referenced this pull request Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants